Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add index on stream_entries table #5793

Merged
merged 1 commit into from Nov 30, 2017
Merged

add index on stream_entries table #5793

merged 1 commit into from Nov 30, 2017

Conversation

takayamaki
Copy link
Contributor

This pull request is adding index for below statement .
SELECT "stream_entries".* FROM "stream_entries" WHERE "stream_entries"."activity_type" = ? AND "stream_entries"."account_id" = ? AND "stream_entries"."hidden" = ? ORDER BY "stream_entries"."id" DESC LIMIT ?

This statement executed 67,112 times.

before

                                                                               QUERY PLAN                                                                               
------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.43..754.44 rows=20 width=48) (actual time=54.970..167.629 rows=20 loops=1)
   ->  Index Scan Backward using index_stream_entries_on_id on stream_entries  (cost=0.43..450939.73 rows=11961 width=48) (actual time=54.968..167.617 rows=20 loops=1)
         Filter: ((NOT hidden) AND ((activity_type)::text = 'Status'::text) AND (account_id = 11109))
         Rows Removed by Filter: 298624
 Planning time: 0.109 ms
 Execution time: 167.658 ms
(6 rows)

after

                                                                   QUERY PLAN                                                                   
------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.43..167.31 rows=20 width=48) (actual time=1.718..5.579 rows=20 loops=1)
   ->  Index Scan Backward using test3 on stream_entries  (cost=0.43..103891.09 rows=12451 width=48) (actual time=1.717..5.572 rows=20 loops=1)
         Index Cond: ((account_id = 11109) AND ((activity_type)::text = 'Status'::text))
         Filter: (NOT hidden)
 Planning time: 0.126 ms
 Execution time: 5.603 ms
(6 rows)

kaniini
kaniini approved these changes Nov 24, 2017
View reviewed changes
Copy link
Contributor

@kaniini kaniini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this sped up timeline queries significantly on my instance

@ykzts ykzts added the performance Runtime performance label Nov 24, 2017
@takayamaki takayamaki force-pushed the add-index-on-stream_entries-table branch from f063861 to d00e15d Compare November 24, 2017 11:15
@Gargron
Copy link
Member

Gargron commented Nov 24, 2017

Isn't a query like this used only on the Atom feed and only when you're paginating it? How could this have improved timelines @kaniini?

@kaniini
Copy link
Contributor

kaniini commented Nov 24, 2017

Sorry, I meant when loading some specific account's timeline.

akihikodaki
akihikodaki previously requested changes Nov 25, 2017
View reviewed changes
db/migrate/20171123163635_add_index_on_stream_entries.rb Outdated
class AddIndexOnStreamEntries < ActiveRecord::Migration[5.1]
def change
commit_db_transaction
add_index :stream_entries, [:id, :account_id, :activity_type], algorithm: :concurrently
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should put id at the last for sorting. By putting account_id at the very beginning you can also cover index_stream_entries_on_account_id and remove it.

@akihikodaki
Copy link
Contributor

Requesting a review from @abcang; he has suggested a similar change for Pawoo, but it is also concerning the idea I suggested in #5793 (review).

@Gargron
Copy link
Member

Gargron commented Nov 25, 2017

I meant when loading some specific account's timeline.

I still don't think this could have any effect on that? As far as I remember account timelines use statuses directly without querying through stream_entries first - only the OStatus Atom feed does that, and it's never accessed programmatically from Mastodon code.

@abcang
Copy link
Contributor

abcang commented Nov 25, 2017

I also recognized that stream_entries is currently used only for atom feed. However, instances that do not correspond to ActivityPub are accessing atom feed, so I think that it will be effective for instances connected to such instances. Also, I think that it is more effective for instances that have many stream_entries like Pawoo.

@takayamaki takayamaki force-pushed the add-index-on-stream_entries-table branch 2 times, most recently from f063861 to b6355bb Compare November 29, 2017 17:27
@takayamaki
Copy link
Contributor Author

Thank you for review.
It is more faster.

mastodon=> explain analyze SELECT "stream_entries".* FROM "stream_entries" WHERE "stream_entries"."activity_type" = 'Status' AND "stream_entries"."account_id" = ***** AND "stream_entries"."hidden" = 'f' ORDER BY "stream_entries"."id" DESC L
IMIT 20;
                                                                  QUERY PLAN                                                                   
-----------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=0.43..78.54 rows=20 width=48) (actual time=0.037..0.078 rows=20 loops=1)
   ->  Index Scan Backward using test6 on stream_entries  (cost=0.43..50676.88 rows=12975 width=48) (actual time=0.035..0.067 rows=20 loops=1)
         Index Cond: ((account_id = 1) AND ((activity_type)::text = 'Status'::text))
         Filter: (NOT hidden)
 Planning time: 0.178 ms
 Execution time: 0.100 ms
(6 rows)

mastodon=>

@takayamaki
Copy link
Contributor Author

This statement was executed 45974 times in the last 6 days at imastodon.net.
If average execution time is 100 ms, it takes 12.77 minutes per day.

I think this is still necessary.

@Gargron Gargron merged commit dc1ebd4 into mastodon:master Nov 30, 2017
@takayamaki takayamaki deleted the add-index-on-stream_entries-table branch November 30, 2017 07:55
cobodo pushed a commit to cobodo/mastodon that referenced this pull request Dec 6, 2017
@takayamaki
add index on stream_entries table (mastodon#5793)
bfc1a67
abcang pushed a commit to pixiv/mastodon that referenced this pull request Dec 18, 2017
@takayamaki @abcang
add index on stream_entries table (mastodon#5793)
73d2a04
abcang added a commit to pixiv/mastodon that referenced this pull request Dec 18, 2017
@abcang
Merge pull request mastodon#730 from pixiv/add_stream_entries_index
f002f27 
add index on stream_entries table (mastodon#5793)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaniini kaniini kaniini approved these changes

@abcang abcang abcang approved these changes

@akihikodaki akihikodaki akihikodaki left review comments

Assignees
No one assigned
Labels
performance Runtime performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@takayamaki @Gargron @kaniini @akihikodaki @abcang @ykzts